-
Notifications
You must be signed in to change notification settings - Fork 70
chore: fix a build error due to name conflict #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: fix a build error due to name conflict #90
Conversation
FetchContent_MakeAvailable generate a <lowercaseName>_SOURCE_DIR[1], for arrow it's arrow_SOURCE_DIR. arrow use a internal arrow_SOURCE_DIR to set its THIRDPARTY_DIR[2]. The generated arrow_SOURCE_DIR overwrite the arrow's interal arrow_SOURCE_DIR, causing the following build error: CMake Error at build/_deps/arrow-src/cpp/cmake_modules/ThirdpartyToolchain.cmake:449 (file): file STRINGS file "/Users/zhjwpku/zhjwpku/iceberg-cpp/build/_deps/arrow-src/thirdparty/versions.txt" cannot be read. Fix this issue by renaming fetchcontent_declare name from Arrow to VendoredArrow. [1] https://cmake.org/cmake/help/latest/module/FetchContent.html#command:fetchcontent_makeavailable [2] https://github.com/apache/arrow/blob/main/cpp/cmake_modules/ThirdpartyToolchain.cmake#L368 Signed-off-by: Junwang Zhao <[email protected]>
wgtmac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Where did you hit this issue?
I just rebased the main branch and removed the build dir then started a new config/build, using the following: I'm wondering why our CI doesn't hit this 😵💫 |
|
I tried to reproduce it but unfortunately it built successfully. |
I think this issue may be caused by a bug in CMake, which appears to have been fixed in version 3.30. However, since we need to support CMake versions back to 3.25, this change seems reasonable. I checked GH runner image[1], the CMake version is 3.31.6, which explains why our CI doesn't trigger the build error. I bet your CMake version is greater than 3.30 ;) (not 100% sure since I'm not familiar their release process, they might have backport the fix to some earlier minor versions). [1] https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md#tools |
|
Ah, that makes sense. Perhaps we can add a comment to your fix so we can restore the Arrow project name once bumping the minimum CMake version? |
Will add a TODO later, thanks |
FetchContent_MakeAvailable generate a _SOURCE_DIR[1], for arrow it's arrow_SOURCE_DIR. arrow use a internal arrow_SOURCE_DIR to set its THIRDPARTY_DIR[2].
The generated arrow_SOURCE_DIR overwrite the arrow's interal arrow_SOURCE_DIR, causing the following build error:
CMake Error at build/_deps/arrow-src/cpp/cmake_modules/ThirdpartyToolchain.cmake:449 (file):
file STRINGS file
"/Users/zhjwpku/zhjwpku/iceberg-cpp/build/_deps/arrow-src/thirdparty/versions.txt"
cannot be read.
Fix this issue by renaming fetchcontent_declare name from Arrow to VendoredArrow.
[1] https://cmake.org/cmake/help/latest/module/FetchContent.html#command:fetchcontent_makeavailable
[2] https://github.com/apache/arrow/blob/main/cpp/cmake_modules/ThirdpartyToolchain.cmake#L368